Skip to content

Implement explicit tail calls in the LLVM backend #138555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

semtexzv
Copy link

Implements guaranteed tail calls in the LLVM backend (#112788):

@rustbot
Copy link
Collaborator

rustbot commented Mar 16, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 16, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

This PR modifies run-make tests.

cc @jieyouxu

@rust-log-analyzer

This comment has been minimized.

@oxalica
Copy link
Contributor

oxalica commented Mar 16, 2025

@semtexzv
Copy link
Author

Looking at the recent comments to understand the GCC backend implementation issue

@rustbot
Copy link
Collaborator

rustbot commented Mar 16, 2025

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@WaffleLapkin
Copy link
Member

r? WaffleLapkin

@WaffleLapkin WaffleLapkin changed the title Implement guaranteed tail calls with the become keyword in the LLVM backend Implement explicit tail calls in the LLVM backend Apr 2, 2025
@WaffleLapkin
Copy link
Member

@semtexzv thanks for your PR!

Do you plan to work on this further?

As @workingjubilee said ~3 weeks ago, the test you've added doesn't actually test anything. The CI is currently failing. I also have a lot of thoughts on the implementation details. All in all there is still quite a bit of work to be done before this can be merged.

Do you have the capacity and desire to continue? I'm a bit concerned that this might be too much for a first time contributor (feel free to prove me wrong tho!)

@rustbot author

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 2, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 2, 2025
@alex-semenyuk
Copy link
Member

@semtexzv
Thanks for your contribution
From wg-triage. Any updates on this PR?

@phi-go
Copy link

phi-go commented Jul 8, 2025

While I would also be a first time contributor, I would be happy to try and implement this. Though, I would only have time to start work on this in two or three weeks.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2025
@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Jul 17, 2025
@rust-log-analyzer

This comment has been minimized.

@semtexzv
Copy link
Author

semtexzv commented Jul 17, 2025

I've updated the implementation to use LLVM's musttail attribute instead of the tail hint.

Specifics:

  1. Added LLVM wrapper for tail call kinds: Created LLVMRustSetTailCallKind in RustWrapper.cpp to access LLVM's setTailCallKind API, supporting all tail call kinds (None, Tail, MustTail, NoTail).

  2. Updated the builder: Modified set_tail_call to use musttail instead of the previous tail hint.

  3. Fixed test issues:

    • Converted the non-functional Makefile test to a proper codegen test
    • Added optimization prevention attributes (#[no_mangle], #[inline(never)], -C no-prepopulate-passes)
    • Created a new test to verify musttail emission in LLVM IR

Known Issue

The tests/ui/explicit-tail-calls/drop-order.rs test causes a SIGSEGV during LLVM optimization passes (specifically in the InlinerPass). This appears to be related to the interaction between musttail and complex control flow involving:

  • RefCell borrowing
  • Drop handlers that mutate borrowed data
  • Nested scopes and loops

The test remains disabled (as it was before) pending further investigation. All other tail call tests pass successfully.

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you accidentally committed an rmeta test file

@@ -1181,6 +1192,7 @@ unsafe extern "C" {
pub(crate) safe fn LLVMIsGlobalConstant(GlobalVar: &Value) -> Bool;
pub(crate) safe fn LLVMSetGlobalConstant(GlobalVar: &Value, IsConstant: Bool);
pub(crate) safe fn LLVMSetTailCall(CallInst: &Value, IsTailCall: Bool);
pub(crate) fn LLVMRustSetTailCallKind(CallInst: &Value, Kind: TailCallKind);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one can be made safe, the caller cannot violate anything that would cause UB

// Call the function
let fn_ty = bx.fn_decl_backend_type(fn_abi);
let fn_attrs = if let Some(instance) = instance
&& bx.tcx().def_kind(instance.def_id()).has_codegen_attrs()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would we ever have a tail call for something that doesn't have codegen attrs?

// We can't test the non-tail version with a large number as it would crash,
// but we can verify it works for small values
assert_eq!(countdown_no_tail(10), 0);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere:

nit: missing trailing newline

Comment on lines +427 to +431
// Perform the actual function call
let llret = bx.call(fn_ty, fn_attrs, Some(fn_abi), fn_ptr, &llargs, None, instance);

// Mark as tail call - this is the critical part
bx.set_tail_call(llret);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we use normal function codegen and just set_tail_call at the end?

Can we at least share some of the argument processing code and instance resolution code?

@@ -595,6 +595,10 @@ pub trait BuilderMethods<'a, 'tcx>:
funclet: Option<&Self::Funclet>,
instance: Option<Instance<'tcx>>,
) -> Self::Value;

/// Mark a call instruction as a tail call (guaranteed tail call optimization)
/// Used for implementing the `become` expression
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All those references to being used to implement become feel a bit redundant to me.

));
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing an sret argument when PassMode::Indirect is used. Also please deduplicate this code with regular calls where possible to prevent divergence.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 17, 2025

Your comments #138555 (comment) and #138555 (comment) are very verbose and hard to process. I don't really have much constructive advice for it

  • I don't understand what the Verification sections are supposed to tell me (it should be obvious that a PR is only exists to actually do the thing it says it does).
  • I don't understand the "fixed compilation error" section (it should be obvious that a PR should compile)

Also your two comments duplicate a lot of information between them. Why are there two comments that partially say the same thing?

This commit implements proper tail call optimization for the `become` expression
by propagating LLVM's musttail attribute, which guarantees tail call optimization
rather than leaving it as an optimization hint.

Changes:
- Add `set_tail_call` method to BuilderMethods trait
- Add FFI wrapper LLVMRustSetTailCallKind to access LLVM's setTailCallKind API
- Implement tail call handling in LLVM backend using musttail
- Implement TailCall terminator codegen in rustc_codegen_ssa
- Make GCC backend fail explicitly on tail calls (not yet supported)
- Add codegen tests to verify musttail is properly emitted
- Add runtime tests for deep recursion and mutual recursion

The musttail attribute is critical for the correctness of the `become`
expression as it guarantees the tail call will be optimized, preventing
stack overflow in recursive scenarios.
@rust-log-analyzer
Copy link
Collaborator

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
[TIMING] core::build_steps::tool::ToolBuild { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu, tool: "tidy", path: "src/tools/tidy", mode: ToolBootstrap, source_type: InTree, extra_features: [], allow_features: "", cargo_args: [], artifact_kind: Binary } -- 9.084
[TIMING] core::build_steps::tool::Tidy { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu } -- 0.000
fmt check
Diff in /checkout/tests/codegen/tail-call-musttail.rs:12:
 pub fn simple_tail_call(n: i32) -> i32 {
     // CHECK: musttail call {{.*}}@simple_tail_call(
     // CHECK-NEXT: ret i32
-    if n <= 0 {
-        0
-    } else {
-        become simple_tail_call(n - 1)
-    }
+    if n <= 0 { 0 } else { become simple_tail_call(n - 1) }
 }
 
 // CHECK-LABEL: define {{.*}}@tail_call_with_args(
Diff in /checkout/tests/codegen/tail-call-musttail.rs:25:
 pub fn tail_call_with_args(a: i32, b: i32, c: i32) -> i32 {
     // CHECK: musttail call {{.*}}@tail_call_with_args(
     // CHECK-NEXT: ret i32
-    if a == 0 {
-        b + c
-    } else {
-        become tail_call_with_args(a - 1, b + 1, c)
-    }
+    if a == 0 { b + c } else { become tail_call_with_args(a - 1, b + 1, c) }
 }
+
fmt: checked 6162 files
Build completed unsuccessfully in 0:00:46
  local time: Thu Jul 17 09:16:46 UTC 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet